-
Notifications
You must be signed in to change notification settings - Fork 0
[JTC] Move PID to controller plugin #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0c2f441 to
71b6efb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll review rest later. So far it looks spot on
| if (use_closed_loop_pid_adapter_) | ||
| { | ||
| trajectory_msgs::msg::JointTrajectory traj; | ||
| traj.points.push_back(state_current_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christophfroehlich Maybe also adding the state_desired_ would help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
computeGains takes a full trajectory, here it is only initialized with the same points as used for set_hold_position
As mentioned in the description, an update for dynamic reconfigure is missing.
But there is also a call missing for receiving a new trajectory, I'll push an update here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a new update
| lock_cmd_joint_names = get_node()->add_pre_set_parameters_callback( | ||
| [this](std::vector<rclcpp::Parameter> & parameters) | ||
| { | ||
| for (auto & parameter : parameters) | ||
| { | ||
| if (parameter.get_name() == "command_joints") | ||
| { | ||
| RCLCPP_ERROR( | ||
| get_node()->get_logger(), | ||
| "The parameter 'command_joints' is read-only. You can't change it."); | ||
| parameter = rclcpp::Parameter("command_joints", command_joint_names_); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christophfroehlich Shouldn't this be set before the set_parameter line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no because I wouldn't be able to set the parameter then. it should just "lock" the parameter afterwards
8b48b19 to
d22d5ff
Compare
de738e6 to
4927435
Compare
Up to now there were only two possibilities of controlling the system with JTC:
The aim of this PR is to create an interface for supporting new control strategies, and enable even creating custom controller plugins by users. This is done by plugins of base class
TrajectoryControllerBase, loaded by the pluginlib.Together with the change of #3 it will even be supported to integrate state-space controller where the system output does not have the same size as the system input: See the following swing-up of a cart-pole from this demo, where I implemented a LQR as plugin
swingup_lqr_gh.mp4
Why to change JTC in this way:
The API was designed around the following methods:
computeControlLawNonRT: called when a new trajectory is received. This may take some time (e.g., calculating the gains for a LQR by solving Riccati equations). This blocks the execution of the new trajectory by JTC until the calculation is finished (seeis_ready()). The user has to implement a realtime-buffer to switch the old control law to the new one, once it is started (seestart()). In case of any abort (update trajectory withset_hold_position()), there is acomputeControlLawRTwhich needs to finish in the RT loop.updateGainsRT: A fast update within the RT loop, e.g., update of the PID gains from dynamic reconfigured ROS parameters.computeCommands: Evaluate the control law inside the RT loop with earlier calculated/updated gains.Notes:
jointsandcommand_jointsparameter #3, the map for the gains should becommand_joints. But most of the time this parameter is empty and copied over fromjointsin JTC. Because it is a static read-only variable, it is not possible to override the parameter for the plugin from JTC -> I had to make it reconfigurable from the generate_parameter_library, but added a callback afterwards to prohibit a later change from the user. If anyone knows a better way to solve this, please let me know.The behavior of existing configurations should not break with the proposed changes.